Skip to content

lib: fix evenRound sign for fractional values in (-1, 1)#62837

Closed
deepview-autofix wants to merge 1 commit intonodejs:mainfrom
deepview-autofix:deepview/500aa18f3b
Closed

lib: fix evenRound sign for fractional values in (-1, 1)#62837
deepview-autofix wants to merge 1 commit intonodejs:mainfrom
deepview-autofix:deepview/500aa18f3b

Conversation

@deepview-autofix
Copy link
Copy Markdown

@deepview-autofix deepview-autofix commented Apr 20, 2026

evenRound derived the rounding direction from MathSign(i) where i = integerPart(x). For any x whose truncated integer part is zero (e.g. 0.7, -0.7), sign was 0, so the non-halfway branch returned i + sign === 0 instead of rounding away from zero.

This produced incorrect [Clamp] WebIDL integer conversions for fractional inputs in (-1, 0) ∪ (0, 1) that are not exactly ±0.5

Observable e.g. via new Blob([Uint8Array.of(1,2,3)]).slice(-0.9999999) returning the whole blob instead of a 1-byte slice.

Use MathSign(x) so the rounding direction follows the sign of the original value.

`evenRound` derived the rounding direction from `MathSign(i)` where
`i = integerPart(x)`. For any `x` whose truncated integer part is zero
(e.g. `0.7`, `-0.7`), `sign` was `0`, so the non-halfway branch returned
`i + sign === 0` instead of rounding away from zero. This produced
incorrect `[Clamp]` WebIDL integer conversions for fractional inputs in
`(-1, 0) ∪ (0, 1)` that are not exactly `±0.5`, observable e.g. via
`new Blob([Uint8Array.of(1,2,3)]).slice(-0.9999999)` returning the whole
blob instead of a 1-byte slice. Use `MathSign(x)` so the rounding
direction follows the sign of the original value.

Co-Authored-By: Claude <[email protected]>
Co-Authored-By: DeepView Autofix <[email protected]>
Co-Authored-By: Nikita Skovoroda <[email protected]>
Signed-off-by: Nikita Skovoroda <[email protected]>
@nodejs-github-bot nodejs-github-bot added the needs-ci PRs that need a full CI run. label Apr 20, 2026
@ChALkeR ChALkeR added the web-standards Issues and PRs related to Web APIs label Apr 20, 2026
Copy link
Copy Markdown
Member

@ChALkeR ChALkeR left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is mine (scanner + autofixer)


Node.js, prior to the patch:

image

Chrome:

image

@ChALkeR ChALkeR marked this pull request as ready for review April 20, 2026 03:55
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 20, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.62%. Comparing base (14e16db) to head (cc729e8).
⚠️ Report is 112 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #62837   +/-   ##
=======================================
  Coverage   89.62%   89.62%           
=======================================
  Files         706      706           
  Lines      219136   219136           
  Branches    41987    41981    -6     
=======================================
+ Hits       196404   196410    +6     
- Misses      14611    14614    +3     
+ Partials     8121     8112    -9     
Files with missing lines Coverage Δ
lib/internal/webidl.js 96.62% <100.00%> (ø)

... and 26 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ChALkeR
Copy link
Copy Markdown
Member

ChALkeR commented Apr 27, 2026

Closing in favor of #62979

@ChALkeR ChALkeR closed this Apr 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-ci PRs that need a full CI run. web-standards Issues and PRs related to Web APIs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants